Skip to content

Security: Fix Local privilege escalation via DLL hijack#46145

Merged
vanzue merged 11 commits intomainfrom
dev/vanzue/fix-sec
Mar 17, 2026
Merged

Security: Fix Local privilege escalation via DLL hijack#46145
vanzue merged 11 commits intomainfrom
dev/vanzue/fix-sec

Conversation

@vanzue
Copy link
Contributor

@vanzue vanzue commented Mar 16, 2026

Summary of the Pull Request

Attack vector:

  1. user install per machine installer
  2. Open an elevated command prompt and verify the newly added PowerToys PATH entry
  3. Inspect the ACL on the DSCModules directory an observe that the "Authenticated Users" group have inherited Modify permissions
  4. Log in as a low-privileged (non-admin) user and confirm that you can create or modify files in C:\PowerToys\DSCModules. This confirms that a non-admin user can plant arbitrary DLLs in a system PATH directory.
  5. The attacker identifies a DLL that a privileged process (e.g., a system service or an application running as a different, higher-privileged user) attempts to load via the standard DLL search order. The attacker crafts a malicious DLL with the same name and places it in C:\PowerToys\DSCModules.

The fix is to:

  • Hardening the PowerToys DSC directory for per-machine custom installs with correct ACL enforced with wix.

PR Checklist

  • Closes: #xxx
  • Communication: I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected
  • Tests: Added/updated and all pass
  • Localization: All end-user-facing strings can be localized
  • Dev docs: Added/updated
  • New binaries: Added on the required places
  • Documentation updated: If checked, please file a pull request on our docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

image

After upgrade, the ACL:

Path : Microsoft.PowerShell.Core\FileSystem::C:\apps\Power Toys\DSCModules
Owner : NT AUTHORITY\SYSTEM
Group : NT AUTHORITY\SYSTEM
Access : CREATOR OWNER Allow 268435456
NT AUTHORITY\SYSTEM Allow FullControl
BUILTIN\Administrators Allow FullControl
BUILTIN\Users Allow ReadAndExecute, Synchronize
Audit :
Sddl : O:SYG:SYD:P(A;OICIIO;GA;;;CO)(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;0x1200a9;;;BU)

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens per-machine custom install locations to mitigate a local privilege escalation vector where permissive inherited ACLs allow non-admin users to plant DLLs in a directory that is added to PATH.

Changes:

  • Adds a new deferred, non-impersonated MSI custom action to secure the install folder ACL for per-machine installs outside Program Files.
  • Implements ACL templating based on Program Files and reapplies hardened ACLs across the existing install tree during upgrade/repair.
  • Wires the new custom action into the WiX installer sequencing for per-machine scenarios.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
installer/PowerToysSetupVNext/Product.wxs Schedules the new SecureInstallFolderAcl custom action and its property setup for per-machine installs.
installer/PowerToysSetupCustomActionsVNext/pch.h Adds ACL API header include needed for the new security logic.
installer/PowerToysSetupCustomActionsVNext/CustomAction.def Exports the new custom action entrypoint for MSI to invoke.
installer/PowerToysSetupCustomActionsVNext/CustomAction.cpp Implements the install-folder ACL hardening + recursive re-ACL logic.

vanzue and others added 4 commits March 16, 2026 11:32
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@vanzue vanzue marked this pull request as draft March 16, 2026 06:30
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this require hundreds of lines of custom action code? does WiX not support ACLs?

@github-actions

This comment has been minimized.

@vanzue vanzue marked this pull request as ready for review March 17, 2026 02:07
@vanzue vanzue requested a review from DHowett March 17, 2026 02:07
@vanzue
Copy link
Contributor Author

vanzue commented Mar 17, 2026

Thanks @DHowett Would you help take a look again see if this is a correct fix?

Copy link
Member

@DHowett DHowett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it could be correct. Can you add a comment to the WXS file explaining the meaning of the SDDL string?

@vanzue
Copy link
Contributor Author

vanzue commented Mar 17, 2026

Done, thanks!!

@vanzue vanzue merged commit 87b24af into main Mar 17, 2026
15 checks passed
@vanzue vanzue deleted the dev/vanzue/fix-sec branch March 17, 2026 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants